Add legal boilerplate validation for external contributors #145
+662
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: vaind <[email protected]>
Co-authored-by: vaind <[email protected]>
Copilot
AI
changed the title
[WIP] Optionally verify presence of required legal notices in PR descriptions
Add legal boilerplate validation for external contributors
Jan 27, 2026
Contributor
|
@copilot as usual, this needs tests... |
Co-authored-by: vaind <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Fix fileContents bug: check for non-empty string instead of catching exceptions - Use author_association field instead of org membership API for reliability - Validate boilerplate content (min 50 chars) not just header presence - Merge tests into dangerfile-utils.test.js following existing patterns - Remove unnecessary undefined arguments from fail() calls All 35 tests pass (20 existing + 15 legal boilerplate tests) Co-authored-by: vaind <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
…ry length Replace arbitrary 50-character check with actual content comparison. Now validates that the exact legal boilerplate from the template is present in the PR description by normalizing whitespace and comparing content. Simplified logic removes unnecessary complexity and ensures the actual legal text stays intact, not just that something exists. Co-authored-by: vaind <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
Extract checkLegalBoilerplate() from dangerfile.js into dangerfile-utils.js
with dependency injection (accepts { danger, fail, markdown } as parameters)
so the orchestration logic can be tested without the DangerJS runtime.
Adds 19 tests covering author association filtering, template discovery,
missing/mismatched boilerplate detection, success cases, and markdown output.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Extract findPRTemplate helper and named constants (INTERNAL_ASSOCIATIONS, PR_TEMPLATE_PATHS). Simplify extractLegalBoilerplateSection using findIndex. Inline checkLegalBoilerplate call in checkAll, removing the wrapper function. Derive test LEGAL_TEXT from the template constant to prevent drift. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Extract normalizeWhitespace, findPRTemplate, and formatBoilerplateHint to module scope for reuse and top-down readability. Deduplicate markdown hint construction across missing/mismatch error paths. Nest integration tests and their helpers inside the top-level describe block. Co-Authored-By: Claude Opus 4.5 <[email protected]>
The test command and Node version requirement were only documented in CI workflow YAML, making it hard for new contributors to discover how to run tests locally. Co-Authored-By: Claude Opus 4.5 <[email protected]>
These helpers were extracted in the previous refactor but lacked direct unit tests, relying only on indirect coverage through the integration tests. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add debug log of the actual author_association value before the internal/external check, making it easier to troubleshoot unexpected association values. Also normalize comment style to /// to match the rest of the file. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Contributor
|
@sentry review |
szokeasaurusrex
approved these changes
Jan 30, 2026
Member
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Nice! Thanks for implementing.
Have you been able to test that it works correctly?
Also, just wondering, is it possible to enable individual checks from the Dangerfile? Like, can I have only the legal boilerplate validation without the changelog validation? I am asking because I am intending to move the Sentry CLI/Sentry Rust repos over to the new Craft changelog generation, which means I will need to disable the Danger checks for the changelog (since manually written changelogs will overwrite Craft's auto-generated changelings). Will it be possible to still have this boilerplate validation?
Contributor
Not outside the PR. If you have a repo at hand you'd like to verify on, you can just point the danger workflow to this PRs latest commit.
Not right now but we can add options |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Add a DangerJS check that validates the presence of the required legal boilerplate in PR descriptions from external contributors.
When a repository's PR template contains a "Legal Boilerplate" section, the check:
author_association)The check is automatically opt-in: repositories without a "Legal Boilerplate" section in their PR template are unaffected.
Known limitation:
author_associationreliabilityThe check uses the GitHub API's
author_associationfield to distinguish internal vs. external contributors. This field is known to be unreliable when an org member has their organization membership visibility set to "Private" (which is the default). In that case,author_associationmay returnNONEorCONTRIBUTORinstead ofMEMBER, causing false positives for internal contributors.A more reliable alternative would be to query the org membership API directly (
GET /orgs/{org}/members/{username}), but that requiresread:orgscope and an extra API call per run. The current approach is a pragmatic starting point — if false positives become a problem, we can switch to the API-based check.See: community discussion #18690, actions/github-script #643
Changes
dangerfile-utils.js: AddextractLegalBoilerplateSection,normalizeWhitespace,findPRTemplate,formatBoilerplateHint, andcheckLegalBoilerplatedangerfile.js: WirecheckLegalBoilerplateintocheckAll()dangerfile-utils.test.js: Add comprehensive tests (~40 cases) covering section extraction, whitespace normalization, hint formatting, and the full orchestration flowREADME.md: Document the new checkCONTRIBUTING.md: Add test runner instructionsValidation against all getsentry repos
Scanned all 300 public, non-archived getsentry repos to verify the check works correctly:
Repos where the check activates
sentry.github/PULL_REQUEST_TEMPLATE.md### Legal Boilerplaterelay.github/PULL_REQUEST_TEMPLATE.md### Legal Boilerplatesnuba.github/PULL_REQUEST_TEMPLATE.md### Legal Boilerplatesentry-cli.github/pull_request_template.md### Legal Boilerplateself-hosted.github/PULL_REQUEST_TEMPLATE.md### Legal Boilerplatevroom.github/PULL_REQUEST_TEMPLATE.md### Legal Boilerplatesentry-docs.github/PULL_REQUEST_TEMPLATE.md## LEGAL BOILERPLATEAll 7 repos use the same underlying legal text.
sentry-docsuses## LEGAL BOILERPLATE(uppercase, h2) and includes an HTML comment before the text — the case-insensitive regex and whitespace normalization handle this correctly.Template path coverage
All discovered templates use one of the 3 paths that
findPRTemplatechecks:.github/PULL_REQUEST_TEMPLATE.md— 17 repos.github/pull_request_template.md— 21 repos.github/PULL_REQUEST_TEMPLATE/pull_request_template.md— 1 repoNo repos use root-level template paths, but they remain supported per GitHub's documentation.
Fixes #144